-
-
Notifications
You must be signed in to change notification settings - Fork 464
feat(anr): Profile main thread when ANR and report ANR profiles to Sentry #4899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
| final AnrProfileManager provider = new AnrProfileManager(options); | ||
| anrProfile = provider.load(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| } | ||
| } catch (IOException e) { | ||
| logger.log(ERROR, "Failed to create stacktrace queue", e); | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrCulpritIdentifier.java
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/anr/AggregatedStackTrace.java
Outdated
Show resolved
Hide resolved
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d5a29b6 | 298.62 ms | 391.78 ms | 93.16 ms |
| 806307f | 357.85 ms | 424.64 ms | 66.79 ms |
| f634d01 | 375.06 ms | 420.04 ms | 44.98 ms |
| 9fbb112 | 361.43 ms | 427.57 ms | 66.14 ms |
| 3d205d0 | 352.15 ms | 432.53 ms | 80.38 ms |
| b77456b | 393.26 ms | 441.10 ms | 47.84 ms |
| b3d8889 | 371.84 ms | 447.49 ms | 75.65 ms |
| a5ab36f | 316.83 ms | 394.54 ms | 77.71 ms |
| ee747ae | 357.79 ms | 421.84 ms | 64.05 ms |
| 27d7cf8 | 309.43 ms | 364.27 ms | 54.85 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d5a29b6 | 1.58 MiB | 2.12 MiB | 549.37 KiB |
| 806307f | 1.58 MiB | 2.10 MiB | 533.42 KiB |
| f634d01 | 1.58 MiB | 2.10 MiB | 533.40 KiB |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| 3d205d0 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| b77456b | 1.58 MiB | 2.12 MiB | 548.11 KiB |
| b3d8889 | 1.58 MiB | 2.10 MiB | 535.07 KiB |
| a5ab36f | 1.58 MiB | 2.12 MiB | 555.26 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 27d7cf8 | 1.58 MiB | 2.12 MiB | 549.42 KiB |
sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrCulpritIdentifier.java
Show resolved
Hide resolved
…ntry-java into markushi/feat/anr-profiling
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
Outdated
Show resolved
Hide resolved
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
Show resolved
Hide resolved
| ### Improvements | ||
|
|
||
| - Do not send manual log origin ([#4897](https://github.com/getsentry/sentry-java/pull/4897)) | ||
| - Add ANR profiling integration ([#4899](https://github.com/getsentry/sentry-java/pull/4899)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 🚫 The changelog entry seems to be part of an already released section
## 8.27.1.
Consider moving the entry to the## Unreleasedsection, please.
| } | ||
| } | ||
| } else { | ||
| options.getLogger().log(SentryLevel.DEBUG, "ANR profile found, but doesn't match"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private @NotNull ParseResult parseThreadDump( | ||
| final @NotNull ApplicationExitInfo exitInfo, final boolean isBackground) { | ||
| final byte[] dump; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: AnrV2Integration.applyAnrProfile() does not call AnrProfileRotationHelper.rotate() after processing, leading to ANR profile data corruption.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The applyAnrProfile() method in AnrV2Integration fails to call AnrProfileRotationHelper.rotate() after reading and deleting the ANR profile file. This breaks the rotation coordination mechanism for subsequent ANR events. As a result, stack traces from multiple ANR events get mixed in the same file, profile data for subsequent ANR events is lost, and potential race conditions can cause data corruption in the QueueFile.
💡 Suggested Fix
Add AnrProfileRotationHelper.rotate() at the end of AnrV2Integration.applyAnrProfile() (around line 388), after deleting the old file and before the method returns.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java#L315-L392
Potential issue: The `applyAnrProfile()` method in `AnrV2Integration` fails to call
`AnrProfileRotationHelper.rotate()` after reading and deleting the ANR profile file.
This breaks the rotation coordination mechanism for subsequent ANR events. As a result,
stack traces from multiple ANR events get mixed in the same file, profile data for
subsequent ANR events is lost, and potential race conditions can cause data corruption
in the `QueueFile`.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 4859262
| for (final @NotNull StackTraceElement element : stack) { | ||
| dos.writeUTF(StringUtils.getOrEmpty(element.getClassName())); | ||
| dos.writeUTF(StringUtils.getOrEmpty(element.getMethodName())); | ||
| dos.writeUTF(StringUtils.getOrEmpty(element.getFileName())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Stack trace file name serialization loses null distinction
When serializing stack trace elements, StringUtils.getOrEmpty(element.getFileName()) converts null file names to empty strings. After deserialization, these become actual empty strings instead of null. In StackTraceConverter.createFrameSignature(), Java string concatenation produces different results for null vs "" - a null file name produces "Class#method#null#10" while an empty string produces "Class#method##10". This causes the frame deduplication logic to treat identical frames (especially native methods which have null file names) as different after a round-trip through persistence, resulting in duplicate frames in the profile.
Additional Locations (1)
| p.close(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing listener removal in close method
The close() method registers itself as an AppStateListener via addAppStateListener(this) in register() but never calls AppState.getInstance().removeAppStateListener(this) when closing. Other integrations in the codebase that implement AppStateListener (like SystemEventsBreadcrumbsIntegration and AndroidConnectionStatusProvider) consistently remove themselves in their close() methods. This omission causes the integration object to remain referenced in the listener list after closure, creating a minor resource leak where the closed integration continues receiving (and ignoring) lifecycle callbacks.
📜 Description
Adds ANR (Application Not Responding) profiling integration that profiles the main thread when an ANR is detected and reports the captured profiles to Sentry.
Key Changes:
AnrProfilingIntegrationto capture profiles during ANR eventsAnrProfileManagerto manage ANR profile lifecycleAnrCulpritIdentifierto identify the culprit code causing ANR💡 Motivation and Context
This feature enables better ANR diagnostics by capturing profiling data at the time of ANR detection, allowing developers to identify performance bottlenecks and problematic code paths causing application hangs.
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps